Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds new scripting macros to increase developer quality of life #5177

Merged
merged 35 commits into from
Oct 12, 2024

Conversation

pkmnsnfrn
Copy link
Collaborator

@pkmnsnfrn pkmnsnfrn commented Aug 15, 2024

Description

Brendan using getobjectfacingdirection to tell what direction the player is facing

  • Adds new scripting macros (removeallitem, getobjectxy, getobjecttemplatexy, getobjectcurrentxy, checkobjectat, setseenmon, setcaughtmon, getseenmon, getcaughtmon, checkspecies, checkspecies_choose, checkspecies_auto, getobjectfacingdirection) to increase developer quality of life

Details

removeallitem

Remove all of specified item from the player's bag.

getobjectxy / getobjecttemplatexy / getobjectcurrentxy

Stores the position of the given object in VAR_0x8007 (x) and VAR_0x8008 (y). Mode CURRENT_POSITION will take the object's current position. Mode TEMPLATE_POSITION will take the object's template position.

checkobjectat

Checks if there is any object at a given x and y position.

setseenmon / setcaughtmon

Checks the state of the Pokédex Seen / Caught flag of the specified Pokemon and stores the result in VAR_RESULT.

getseenmon / getcaughtmon

Sets the Pokédex Seen / Caught flag of the specified Pokemon.

checkspecies / checkspecies_choose / checkspecies_auto

Check if the Player has \speciesId in their party. OPEN_PARTY_SCREEN will have the player select a mon from their party. NO_PARTY_SCREEN will automatically check every mon in the player's party.

getobjectfacingdirection

Gets the facing direction of a given event object and stores it in the given variable.

Testing

Clean Branch

You can recreate this branch by applying a patch or pulling the repo. From a clean version of expansion's upcoming, you can either:

Patch

wget https://files.catbox.moe/031oqa.patch -O script.patch ; git apply script.patch ; rm script.patch

Repo

git remote add psf-expansion https://github.com/PokemonSanFran/pokeemerald-expansion/ ; git pull psf-expansion scriptCommands

Manual Tests

After replicating the branch, to recreate my testing environment, you can either directly download the debug script, or manually create the changes.

Download

wget https://files.catbox.moe/v2vl7q.inc -O data/scripts/debug.inc

Manual Testing

  • Modify data/scripts/debug.inc to the match given debug script
  • Compile
  • Start new game
  • Exit truck and enter house
  • Run Cheat Start
  • Run desired scripts

Verified Scenarios

All verified scenarios follow the flow described in Manual Testing.

removeallitem (Script 1 and Script 2)

removeallitem.mp4

getobjectxy (Script 3)

getobjectxy.mp4

checkobjectat (Script 4)

checkobjectat.mp4

getseenmon / setseenmon / setcaughtmon (Script 5)

getseenmon.mp4

checkspecies_auto (Script 7)

checkspecies_auto.mp4

getobjectfacingdirection (Script 8)

getobjectfacingdirection.mp4

People who collaborated with me in this PR

These macros were written by @Pawkkie, @Lunos, and @ghoulslash and were originally part of Useful Scripting Specials. Give them all the credit.

Discord Contact Info

I am pkmnsnfrn on Discord.

@pkmnsnfrn pkmnsnfrn added new-feature Adds a feature category: overworld Pertains to out-of-battle mechanics labels Aug 15, 2024
@AlexOn1ine
Copy link
Collaborator

Does it make sense to add new specials? They make devs lives easier but a lot of them are also subjective meaning why would you add X but not Y?

@ghoulslash
Copy link
Collaborator

Does it make sense to add new specials? They make devs lives easier but a lot of them are also subjective meaning why would you add X but not Y?

When script cmds take arguments I personally always prefer a callnative. With specials you have to use scripting vars for Args which has the (rare) potential to stomp on other values you've set previously in the script.

@AlexOn1ine
Copy link
Collaborator

Does it make sense to add new specials? They make devs lives easier but a lot of them are also subjective meaning why would you add X but not Y?

When script cmds take arguments I personally always prefer a callnative. With specials you have to use scripting vars for Args which has the (rare) potential to stomp on other values you've set previously in the script.

The same question applies. Would it make sense to add them? Just trying to figure out scope on this one

@pkmnsnfrn
Copy link
Collaborator Author

pkmnsnfrn commented Aug 15, 2024

Does it make sense to add new specials? They make devs lives easier but a lot of them are also subjective meaning why would you add X but not Y?

Is there a good reason not to add as many as we want? There's no scrolling list or menu with finite space. Space isn't an issue, we can switch to callnative if the special table is finite.

I was inspired to add this once 4189 was merged. If we can add things that would improve quality of life, we should.

When script cmds take arguments I personally always prefer a callnative. With specials you have to use scripting vars for Args which has the (rare) potential to stomp on other values you've set previously in the script.

ghoul two of these are yours and they're not callnative
If wrapping all of these in functions that use scriptcontext and callnative is blocking, I think that's fair and I'm happy to do it.

@AlexOn1ine
Copy link
Collaborator

AlexOn1ine commented Aug 15, 2024

I was inspired to add this once #4189 (review). If we can add things that would improve quality of life, we should.

I'm generally wondering what is going to be useful for other projects because I would guess most of the time people write their own specific specials that only make sense for their projects. So trying to avoid the trap where we accept each random special basically answering the question why X but not Y.

@pkmnsnfrn
Copy link
Collaborator Author

I edited my post but I don't think you saw it in time. In any case, I think we all agree that a command for a specific scenario in one person's game should not be merged.

If you're looking for a guiding principle, I trust the (current) Senate enough to say:

If another Senate member thinks "yeah, this could be generically useful"

then it should be merged.

it is mere coincidence that these were all written by current Senate members and therefore passes that bar

@Bassoonian
Copy link
Collaborator

I personally think setmonball doesn't qualify since it's a non-vanilla feature, I'm fine with the others. For removeallitem it may be useful to return (through VAR_RESULT) the amount of removed items for an easy addition of further logic. I also don't think the checkspecies variants need to exist; rather, one of the two could be set as the default with the other needing to be explicitly set through the argument.

@pkmnsnfrn
Copy link
Collaborator Author

@Bassoonian restating for clarity

merge criteria is

  1. Remove setmonball
  2. Set VAR_RESULT to the number of removed items from removallitem
  3. Remove checkspecies_auto and make its behavior the default, with the ability to use checkspecies_choose
  4. still waiting on an answer to callnative discussion from above

@pkmnsnfrn
Copy link
Collaborator Author

@ghoulslash - was your feedback saying "convert the specials to callnative"?

@pkmnsnfrn pkmnsnfrn marked this pull request as draft August 28, 2024 03:00
@pkmnsnfrn pkmnsnfrn marked this pull request as ready for review September 14, 2024 15:58
@pkmnsnfrn
Copy link
Collaborator Author

All feedback addressed.

@ghoulslash ghoulslash merged commit f810d7d into rh-hideout:upcoming Oct 12, 2024
1 check passed
@Bassoonian Bassoonian mentioned this pull request Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: overworld Pertains to out-of-battle mechanics new-feature Adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants